Skip to content

Simplify esphome#22868

Merged
balloob merged 8 commits into
home-assistant:devfrom
OttoWinter:simplify-esphome
Apr 16, 2019
Merged

Simplify esphome#22868
balloob merged 8 commits into
home-assistant:devfrom
OttoWinter:simplify-esphome

Conversation

@OttoWinter
Copy link
Copy Markdown
Member

@OttoWinter OttoWinter commented Apr 7, 2019

Description:

Merge #22859 first - see 94f8c4f for changes made for this PR

Simplify the esphome integration code by refactoring common patterns into decorators.

  • esphome_state_property - Wrap a state property (that accesses self._state) by checking for None state and NAN values.
  • esphome_map_enum - Cleans up some code in FAN/CLIMATE where enums need to be converted between HA str and esphome int representation. Prevents declaring the same constants twice each time.

Also adds some more type hints in places.

Checklist:

  • The code change is tested and works locally.
  • Local tests pass with tox. Your PR cannot be merged unless tests pass
  • There is no commented out code in this PR.

@codecov

This comment has been minimized.

@balloob
Copy link
Copy Markdown
Member

balloob commented Apr 15, 2019

Can you rebase so we can merge?

# Conflicts:
#	homeassistant/components/esphome/climate.py
@OttoWinter
Copy link
Copy Markdown
Member Author

OttoWinter commented Apr 16, 2019

It appears like codecov doesn't like merge commits - manual output checking reveals the coverage has actually not changed at all. (Probably codecov is just not calculating the merge base correctly).

Still ok to merge @balloob ?

btw, can we disable codecov comments please? They are just so spammy and take up so much space in the comments. Plus I don't see any added value over just a simple CI check in the "merge" box at the bottom of the PR. Even g suite (correctly) moves these e-mails in the spam folder automatically :D

@balloob balloob merged commit 3186109 into home-assistant:dev Apr 16, 2019
@balloob
Copy link
Copy Markdown
Member

balloob commented Apr 16, 2019

Yeah, probably better to disable them. Don't think many people are looking at them.

@balloob
Copy link
Copy Markdown
Member

balloob commented Apr 16, 2019

Want to open a PR for that?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants